[Cloud Security] Removing license gate keeping and displaying the table when there are findings by JordanSh · Pull Request #190285 · elastic/kibana

JordanSh · 2024-08-11T15:59:52Z

Summary

Resolves https://github.com/elastic/security-team/issues/9939
Resolves [Cloud Security] Add license gate to integration page #189855
Removed license checks from the CloudPosturePage component
Now displays findings if there are any in the CDR index, regardless to CSPM/KSPM status
Now checks license on integration page, displays UI block and disable policy saving
License blocks now specify which license is needed
Fixed tests according to these changes

Findings are displayed without CSP integration installed

Screen.Recording.2024-08-12.at.15.54.18.mov

License check moved to integration page

Screen.Recording.2024-08-12.at.15.56.19.mov

Skips license block on serverless:

…led but there are findings

JordanSh · 2024-08-12T12:52:48Z

/ci

JordanSh · 2024-08-12T14:26:51Z

/ci

elasticmachine · 2024-08-12T16:06:33Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

JordanSh · 2024-08-13T10:01:08Z

  </div>
 );

+const useEnsureDefaultNamespace = ({


here to line 661 did not change, i just moved it to the top because this file is hard enough to read, having utility functions both at the top of the file and at its bottom was making it really confusing

maxcold · 2024-08-14T13:01:19Z


 const SUBSCRIPTION_QUERY_KEY = 'csp_subscription_query_key';

-export const useSubscriptionStatus = () => {


nit: if we rename the only hook, should we also rename the file? not like we strictly follow one hool per file policy, so just a nit

maxcold · 2024-08-14T13:12:29Z

  const location = useLocation();
  const dataViewQuery = useDataView(CDR_MISCONFIGURATIONS_DATA_VIEW_ID_PREFIX);
  const { data: getSetupStatus, isLoading: getSetupStatusIsLoading } = useCspSetupStatusApi();
+  const getLatestFindings = useLatestFindings({ sort: [['asc']], enabled: true, pageSize: 1 });


what about making this a part of the /status API? I think we will need this check in many places, one I know about is the Contextual Flyout for the empty. I think it's better to have a well-defined API for that rather than doing it in every place again and again

plus there is a check for muted/unmuted findings integrated in the useLatestFindings and I'm not sure if we want it here. If I'm not missing smth, if there are findings but all muted, this page would show the "install CSPM integration" block, while the integration is installed. Is this correct?
For me, this is another pro of having this as a part of the status API. there we can encapsulate pure statuses, without the risk of the logic being affected by the changes in the useLatestFindings hook

I was having the same contemplation regarding the muted rules. One thing to keep in mind is that we only support rule muting for native findings. Meaning CDR rules will never have filtered out findings, at least on paper.
Meaning that using the current useLatestFindings is basically keeping the same logic that we always had without changing any flow, which is the same as you mentioned, all native findings muted -> integration installation prompt. Ideally we should have some "all findings are muted" prompt or something, but we don't have that yet.

I was also having the same contemplation regarding the status API tbh, ended up with what you see cause I figured I want to use exactly what the table is using so there's no chance of the API saying "have findings" while the table is like "nope". or vise versa somehow. I can't think of something that can cause that right now, but it might come up in the future and it's just an additional thing to keep in mind. I'm ok with changing it to use an API, lmk if you still think its better after i explained my reasoning

I am in favor of defining a clear status API and then combining them in the react components according to the use case. If we need to reuse some logic between the data grid and the page itself, we can always come up with an abstraction for it. Right now there is an implicit coupling between different pages, plus there are side effects like muting logic which might not be desirable, even if for now we are ok with it

…bana into removing-gatekeepers

opauloh · 2024-08-15T22:14:48Z

+            {
+              range: {
+                '@timestamp': {
+                  gte: `now-${LATEST_FINDINGS_RETENTION_POLICY}`,


should we also include the retention policy as a function parameter? so the function will be truly generic to other use cases such as vulnerabilities.

opauloh

just a comment, LGTM overall!

maxcold · 2024-08-19T11:50:05Z

-    hasFindings ||
+  const hasFindings =
+    hasMisconfigurationsFindings ||
    getSetupStatus?.kspm.status === 'indexed' ||


nit: I think getSetupStatus?.kspm.status === 'indexed' || getSetupStatus?.cspm.status === 'indexed' check is not needed anymore, I can't think of a scenario when there are no documents in the CDR index pattern, but either kspm or cspm is indexed

maxcold · 2024-08-19T11:52:06Z

    installedPackagePoliciesVulnMgmt,
    installedPolicyTemplates,
  ] = await Promise.all([
+    checkIndexHasFindings(esClient, CDR_MISCONFIGURATIONS_INDEX_PATTERN, logger),


let's add an API integration test for the new piece of information.

maxcold

Looks good, also tested on a security serverless project.

…g-gatekeepers

kibana-ci · 2024-08-19T16:44:25Z

💚 Build Succeeded

Buildkite Build
Commit: 2deb1a5
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-190285-2deb1a5479b9
Security Deployment

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`cloudSecurityPosture`	477.9KB	478.0KB	+113.0B

History

💔 Build #228282 failed 428cfdc
💚 Build #228231 succeeded f375ca8
💔 Build #227972 failed 5b6d053
💛 Build #227626 was flaky 288e632

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @JordanSh

JordanSh added 3 commits August 11, 2024 16:25

init

36fcf77

shows cdr findings if they exist

f74081c

added test for displaying the table while integrations are not instal…

868f31c

…led but there are findings

JordanSh added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Cloud Security Cloud Security team related labels Aug 11, 2024

JordanSh self-assigned this Aug 11, 2024

JordanSh added 2 commits August 12, 2024 14:10

before removing from tests

5e45d76

tests

fd9d240

added serverless check only

fea2bb3

merged main

89c8b9b

JordanSh marked this pull request as ready for review August 12, 2024 16:06

JordanSh requested a review from a team as a code owner August 12, 2024 16:06

JordanSh commented Aug 13, 2024

View reviewed changes

maxcold added the ci:project-deploy-security Create a Security Serverless Project label Aug 14, 2024

maxcold reviewed Aug 14, 2024

View reviewed changes

maxcold and others added 3 commits August 14, 2024 15:22

Merge branch 'main' into removing-gatekeepers

288e632

review changes

6118d2b

Merge branch 'removing-gatekeepers' of https://github.com/JordanSh/ki…

5b6d053

…bana into removing-gatekeepers

opauloh reviewed Aug 15, 2024

View reviewed changes

opauloh approved these changes Aug 15, 2024

View reviewed changes

maxcold reviewed Aug 19, 2024

View reviewed changes

maxcold approved these changes Aug 19, 2024

View reviewed changes

JordanSh added 3 commits August 19, 2024 15:31

fixes

7b1cee3

Merge branch 'main' of https://github.com/elastic/kibana into removin…

f375ca8

…g-gatekeepers

added tests

2b179e1

JordanSh added 2 commits August 19, 2024 17:25

added tests

428cfdc

fix index name

2deb1a5

JordanSh enabled auto-merge (squash) August 19, 2024 16:12

JordanSh merged commit d0c1349 into elastic:main Aug 19, 2024

kibanamachine added the v8.16.0 label Aug 19, 2024

JordanSh mentioned this pull request Mar 26, 2025

Check license on plugin setup #160348

Open


		const SUBSCRIPTION_QUERY_KEY = 'csp_subscription_query_key';

		export const useSubscriptionStatus = () => {

Conversation

JordanSh commented Aug 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Findings are displayed without CSP integration installed

License check moved to integration page

Skips license block on serverless:

Uh oh!

JordanSh commented Aug 12, 2024

Uh oh!

JordanSh commented Aug 12, 2024

Uh oh!

elasticmachine commented Aug 12, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxcold Aug 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

opauloh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxcold left a comment

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Metrics [docs]

Async chunks

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JordanSh commented Aug 11, 2024 •

edited

Loading

maxcold Aug 15, 2024 •

edited

Loading

kibana-ci commented Aug 19, 2024 •

edited

Loading